refactor(routes): move vault domain into routes/vault/ subpackage - #5780
Conversation
github-actions
Bot
added
the
ready for review
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Friendly ping for review 👋 This is one of three parallel low-risk slices (#5779 search, #5780 vault, #5781 webhook). Only the vault domain moves — a single-file, 242-line domain with zero internal Both points from your #4903 review are baked in (canonical doesn't depend on shim; shim regression test included). Note on CI: The @RaresKeY @alteixeira20 — would you mind taking a look when you have a moment? |
RaresKeY
left a comment
RaresKeY
left a comment
There was a problem hiding this comment.
Thanks for the update. I checked the latest head and found one scope issue that should be addressed before merge.
Findings
-
Problem: This PR targets
dev, but its commit list begins with the complete search relocation from #5779 before the vault commit. The full nine-file diff changes bothroutes/search/androutes/vault/, while the readiness comment says only the vault domain moves. -
Impact: Squash-merging this PR would land search and vault together under a vault-only review unit, duplicate or supersede #5779, and invalidate the intended per-domain check and review boundary.
-
Ask: Please rebuild the branch by cherry-picking the vault commit directly onto the latest
devso this PR contains only the vault relocation, then rerun checks and request a fresh review. -
Location: PR commit list and file list;
app.py:695changes the search import andapp.py:855changes the vault import.
Validation
- Compared the full two-commit diff with the vault-only second-commit delta.
- Confirmed the vault implementation itself remains byte-for-byte preserved and its focused route/shim tests pass on a current-
devcomposition.
2033ff2 to
4775f22
Compare
Slice 2k of the route-domain reorganization (odysseus-dev#4082/odysseus-dev#4071). Moves vault_routes.py into routes/vault/, leaving a backward-compat sys.modules shim. Pure file reorganization, no behavior change.
4775f22 to
8c28637
Compare
|
Thanks @RaresKeY — good catch on the scope issue. I rebuilt the branch by cherry-picking the vault/webhook commit directly onto the latest Confirmed: the commit list shows a single commit, and the file diff touches only |
|
Hi @RaresKeY — following up on the scope issue you flagged. The branch has been rebuilt to contain only the vault relocation (single commit, no search changes — see file list). CI is fully green. Just needs a re-review whenever you have a moment. Thanks! |
Brings upstream's two route-subpackage moves (odysseus-dev#5781 routes/webhook/, odysseus-dev#5780 routes/vault/) and the integration re-baseline commit. Both moves follow the established shim pattern and the fork has zero divergence in either domain, so this is the first promotion since integration was restored to the documented flow.
…ysseus-dev#5780) Slice 2k of the route-domain reorganization (odysseus-dev#4082/odysseus-dev#4071). Moves vault_routes.py into routes/vault/, leaving a backward-compat sys.modules shim. Pure file reorganization, no behavior change.
3 participants
Summary
Slice 2k of the route-domain reorganization (Refs #4082 / #4071). Moves the vault domain (242 lines) into
routes/vault/behind asys.modulesshim. Pure file reorganization, no behavior change.One of three parallel low-risk slices (search, vault, webhook). The
sys.modulesshim handles theimport ... as vr+monkeypatch.setattrpattern. Zero path-string landmines. Canonical module imports only fromcore/,src/, and stdlib.How to test
Target branch
dev, notmain.Linked Issue
Follows the Phase 0 architecture refactor (#4082 / #4071). One domain per PR.
Type of Change
Checklist
dev